Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Restarting cilium daemonset on upgrade workflow to address upgrade issue #2066

Closed
wants to merge 1 commit into from
Closed

Restarting cilium daemonset on upgrade workflow to address upgrade issue #2066

wants to merge 1 commit into from
+109 −1

Conversation

maxdrib
Copy link
Contributor

@maxdrib maxdrib commented May 9, 2022

Issue #, if available:
#1888

Description of changes:
This PR restarts the cilium pods in a CloudStack upgrade workflow in order to prevent the issues observed in #1888. We are still waiting to hear what the root cause may be from Isovalent but expect this change to resolve the majority of the issues.

Created #2061 to make the logic non Cloudstack-specific

This PR was merged in main but the changes should be included in the upcoming release

Testing (if applicable):
Unit tests. Executed the TestCloudStackKubernetes120RedhatTo121Upgrade locally several times and observed the networking issues recover during the upgrade

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@maxdrib
Restarting cilium daemonset on upgrade workflow to address upgrade is…
8d734fa 
…sue (#2057)

* Restarting cilium daemonset on upgrade workflow to address #1888

* moving cilium restart to cluster manager

* Fixing unit tests

* Improving unit test coverage

* Renaming daemonset restart method to be generic

* Renaming tests to decouple from cilium
@eks-distro-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign mdsgabriel after the PR has been reviewed.
You can assign the PR to them by writing /assign @mdsgabriel in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@eks-distro-bot eks-distro-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 9, 2022
@codecov
Copy link

codecov bot commented May 9, 2022 •
edited
Loading

Codecov Report

Merging #2066 (8d734fa) into release-0.9 (9a56d9d) will increase coverage by 0.02%.
The diff coverage is 82.35%.

@@               Coverage Diff               @@
##           release-0.9    #2066      +/-   ##
===============================================
+ Coverage        49.59%   49.61%   +0.02%     
===============================================
  Files              308      308              
  Lines            25894    25911      +17     
===============================================
+ Hits             12842    12856      +14     
- Misses           11732    11734       +2     
- Partials          1320     1321       +1
Impacted Files Coverage Δ
pkg/clustermanager/cluster_manager.go 53.50% <57.14%> (+0.03%) ⬆️
pkg/executables/kubectl.go 37.81% <100.00%> (+0.57%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9a56d9d...8d734fa. Read the comment docs.

@maxdrib
Copy link
Contributor Author

maxdrib commented May 9, 2022

Closing in favor of #2068, which was automatically generated with cherrypick command

@maxdrib maxdrib closed this May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@maxdrib @eks-distro-bot